-
Notifications
You must be signed in to change notification settings - Fork 316
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixes handling of deferred payments (ask to buy) #296
Fixes handling of deferred payments (ask to buy) #296
Conversation
@@ -1039,7 +1039,26 @@ - (void)storeKitWrapper:(RCStoreKitWrapper *)storeKitWrapper | |||
} | |||
break; | |||
} | |||
case SKPaymentTransactionStateDeferred: | |||
case SKPaymentTransactionStateDeferred: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks great 👍 I left a couple of suggestions
Purchases/Public/RCPurchases.m
Outdated
NSError *pendingError = [NSError errorWithDomain:RCPurchasesErrorDomain | ||
code:RCPaymentPendingError | ||
userInfo:@{ | ||
NSLocalizedDescriptionKey: @"The payment is deferred." | ||
}]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe this should live in https://github.com/RevenueCat/purchases-ios/blob/develop/Purchases/Public/RCPurchasesErrorUtils.h
Purchases/Public/RCPurchases.m
Outdated
pendingError, | ||
transaction.error.code == SKErrorPaymentCancelled); | ||
@synchronized (self) { | ||
self.purchaseCompleteCallbacks[transaction.payment.productIdentifier] = nil; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we should start checking if transaction
, transaction.payment
and transaction.payment.productIdentifier
aren't nil before doing this. otherwise we could run into issues like ##279
On another note, we might not need to make two locks - why not unify the synchronize calls?
@synchronized (self) {
completion = self.purchaseCompleteCallbacks[transaction.payment.productIdentifier];
self.purchaseCompleteCallbacks[transaction.payment.productIdentifier] = nil;
}
the block should be copied anyway, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very good point! Regarding the blocks, yes, I think we can combine the synchronized blocks
…nst transaction.payment.productIdentifier
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks great!
Should fix #273
In order to test it add:
to the
SKMutablePayment
. This flag will trigger aSKPaymentTransactionStateDeferred
. The flow in sandbox is not the same as in production. In production, guardians get a notification with an approval request. In sandbox I have only managed to get thatSKPaymentTransactionStateDeferred
transaction.Please note that we are not finishing the transaction. The documentation is not clear, but looking at https://developer.apple.com/documentation/storekit/in-app_purchase/offering_completing_and_restoring_in-app_purchases, it looks like we only need to "Call finishTransaction(_:) on transactions whose state is SKPaymentTransactionState.failed, SKPaymentTransactionState.purchased, or SKPaymentTransactionState.restored to remove them from the queue."